Skip to content

✨ Deletion of trashed projects/folders upon expiration of retention time #7246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 44 commits into from
Feb 20, 2025
Merged

✨ Deletion of trashed projects/folders upon expiration of retention time #7246

merged 44 commits into from
Feb 20, 2025

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Feb 18, 2025 •
edited
Loading

What do these changes do?

This PR is a continuation of the work on the trash feature. Specifically:

  • ✨ delete expired trashed items as "admin-user" (i.e. w/o access-rights check):
    • implemented in trash._service.delete_expired_trash
    • expose api for background task (trash.trash_service)
  • ✨ background task runs periodically in the webserver-GC service to delete trashed items over the retention time
    • implemented as a task in the GC service (uses legacy style for bg tasks for now as the rest in the GC)
    • retention time given by TRASH_RETENTION_DAYS
    • periodicity of GC for this task give by GARBAGE_COLLECTOR_PRUNE_APIKEYS_INTERVAL_S (for now)
  • Misc
    • ♻️ refactoring feature example
    • 🔨 new test functions for Unset
    • 🔨 enhances test errors
    • 🐛♻️ Added the TRASH_RETENTION_DAYS missing env var to the webserver-GC option and sorted env-vars in the docker-compose

Next steps

Related issue/s

  • part of ITISFoundation/private-issues#6

How to test

cd services/web/service
make install-dev
pytest -vv -k test_trash_service__delete_expired_trash tests/unit/with_db 
pytest -vv tests/unit/with_db/**/test_*trash*.py

Dev-ops

  • changes in osparc-config not needed

@pcrespov pcrespov self-assigned this Feb 18, 2025
@codecov
Copy link

codecov bot commented Feb 18, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 86.84211% with 30 lines in your changes missing coverage. Please review.

Project coverage is 87.77%. Comparing base (ba9624a) to head (2548ba8).
Report is 1 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #7246      +/-   ##
==========================================
+ Coverage   87.08%   87.77%   +0.68%     
==========================================
  Files        1679     1671       -8     
  Lines       64869    64056     -813     
  Branches     1115     1140      +25     
==========================================
- Hits        56491    56222     -269     
+ Misses       8060     7509     -551     
- Partials      318      325       +7
Flag Coverage Δ
integrationtests 65.46% <54.22%> (+0.03%) ⬆️
unittests 86.07% <83.77%> (-0.05%) ⬇️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library 94.17% <ø> (ø)
pkg_dask_task_models_library 97.09% <ø> (ø)
pkg_models_library 91.54% <100.00%> (ø)
pkg_notifications_library 84.57% <ø> (ø)
pkg_postgres_database 88.28% <ø> (ø)
pkg_service_integration 70.03% <ø> (ø)
pkg_service_library 72.61% <ø> (ø)
pkg_settings_library 90.61% <ø> (ø)
pkg_simcore_sdk 85.46% <ø> (+0.37%) ⬆️
agent 96.46% <ø> (ø)
api_server 90.56% <ø> (ø)
autoscaling 96.08% <ø> (ø)
catalog 91.71% <ø> (ø)
clusters_keeper 99.24% <ø> (ø)
dask_sidecar 91.25% <ø> (ø)
datcore_adapter 93.19% <ø> (ø)
director 76.59% <ø> (-0.10%) ⬇️
director_v2 91.29% <ø> (+0.01%) ⬆️
dynamic_scheduler 97.33% <ø> (ø)
dynamic_sidecar 89.74% <ø> (-0.04%) ⬇️
efs_guardian 90.25% <ø> (ø)
invitations 93.28% <ø> (ø)
osparc_gateway_server ∅ <ø> (∅)
payments 92.66% <ø> (ø)
resource_usage_tracker 89.13% <ø> (-0.06%) ⬇️
storage 86.73% <ø> (+0.05%) ⬆️
webclient ∅ <ø> (∅)
webserver 87.50% <87.11%> (+2.49%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba9624a...2548ba8. Read the comment docs.

@pcrespov pcrespov force-pushed the is468/rm-expired-trash branch 2 times, most recently from 2125de1 to 861b2f2 Compare February 18, 2025 22:26
@pcrespov pcrespov added this to the Singularity milestone Feb 19, 2025
@pcrespov pcrespov added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Feb 19, 2025
@pcrespov pcrespov force-pushed the is468/rm-expired-trash branch 2 times, most recently from 6a2e54f to 5d9fc81 Compare February 19, 2025 16:27
@pcrespov pcrespov marked this pull request as ready for review February 20, 2025 09:27
@pcrespov pcrespov force-pushed the is468/rm-expired-trash branch from 58116c4 to 9da3764 Compare February 20, 2025 09:27
@pcrespov pcrespov changed the title WIP: ✨ Is468/rm expired trash Deletion of trashed items upon expiration of retention time Feb 20, 2025
@pcrespov pcrespov changed the title Deletion of trashed items upon expiration of retention time ✨ Deletion of trashed items upon expiration of retention time Feb 20, 2025
@pcrespov pcrespov changed the title ✨ Deletion of trashed items upon expiration of retention time ✨ Deletion of trashed projects/folders upon expiration of retention time Feb 20, 2025
@matusdrobuliak66
Copy link
Collaborator

matusdrobuliak66 commented Feb 20, 2025

Thanks!

GitHK
GitHK reviewed Feb 20, 2025
View reviewed changes
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: where is this ran, in which service? The webserver or the GC?
How do you manage to make this only run in one copy?

@pcrespov
Copy link
Member Author

pcrespov commented Feb 20, 2025 •
edited
Loading

Q: where is this ran, in which service? The webserver or the GC? How do you manage to make this only run in one copy?

This is what I tried to explain in the description :-) HEre some concrete answers

Q: where is this ran, in which service?

the background is in the webserver-GC (see tasks_trash)

How do you manage to make this only run in one copy?

There is only one replica of the webserver-GC. The tasks of this service suffer from this issue as well, so I plan to upgrade them all (but in a separate PR, otherwise it is messy)
For now, as I mentioned above, this PR uses the current background task mechanism already in the GC

@pcrespov pcrespov force-pushed the is468/rm-expired-trash branch from c6a81f8 to 1433f19 Compare February 20, 2025 15:46
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 20, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@pcrespov pcrespov enabled auto-merge (squash) February 20, 2025 17:29
@pcrespov pcrespov merged commit 7be787f into ITISFoundation:master Feb 20, 2025
87 of 95 checks passed
@pcrespov pcrespov deleted the is468/rm-expired-trash branch February 20, 2025 17:31
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Mar 6, 2025
Closed
63 tasks
mrnicegyu11 pushed a commit to mrnicegyu11/osparc-simcore that referenced this pull request Mar 26, 2025
@pcrespov @mrnicegyu11
✨ Deletion of trashed projects/folders upon expiration of retention t…
02170ee 
…ime (ITISFoundation#7246)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@odeimaiz odeimaiz odeimaiz left review comments

@GitHK GitHK GitHK approved these changes

@YuryHrytsuk YuryHrytsuk YuryHrytsuk approved these changes

@matusdrobuliak66 matusdrobuliak66 matusdrobuliak66 approved these changes

@sanderegg sanderegg Awaiting requested review from sanderegg sanderegg is a code owner

@giancarloromeo giancarloromeo Awaiting requested review from giancarloromeo giancarloromeo is a code owner

@bisgaard-itis bisgaard-itis Awaiting requested review from bisgaard-itis

@mrnicegyu11 mrnicegyu11 Awaiting requested review from mrnicegyu11 mrnicegyu11 is a code owner

Assignees

@pcrespov pcrespov

Labels

a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Milestone

Singularity

Development

Successfully merging this pull request may close these issues.

7 participants

@pcrespov @matusdrobuliak66 @giancarloromeo @GitHK @odeimaiz @sanderegg @YuryHrytsuk